Fix "wait for all" not resolving when given empty list of promises #58
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I think there could be a problem here. If I'm reading it correctly, we're calling successSteps synchronously when the list is empty, but asynchronously in all other cases. Maybe it needs to be something like "queue a microtask to run successSteps"? |
domenic
requested changes
Jul 16, 2019
|
Would this work?
I feel like it might be confusing whether "and return" is still part of the outer algorithm, or whether it's part of the steps inside the microtask. Would you prefer I change it to this instead?
|
|
Nested steps seems slightly better; thanks! I also need to investigate the build failures, hrm. |
MattiasBuelens
commented
Jul 16, 2019
| @@ -269,6 +272,9 @@ To <dfn id="waiting-for-all" export lt="wait for all|waiting for all">wait for a | |||
| 1. Let |index| be 0. | |||
| 1. Let |total| be |promises|'s [=list/size=]. | |||
| 1. Let |result| be a <a>list</a> containing |total| null values. | |||
| 1. If |total| is 0, then: | |||
| 1. <a>Queue a microtask</a> to perform |successSteps| given |result|. | |||
There was a problem hiding this comment.
I hope I'm doing this autolinking right! 😅
I still don't know when to use "dfn autolinks" ([= ... =]) and when to use manual <a> elements though... 🤷♂
There was a problem hiding this comment.
They're equivalent. Basically [=...=] is newer so some older specs like this one still use <a>. (And also sometimes newer specs written by people with older habits.)
domenic
approved these changes
Jul 16, 2019
domenic
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Jul 16, 2019
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Jul 24, 2019
…eventCancel and preventAbort works, a=testonly Automatic update from web-platform-tests Streams: verify that aborting a pipe with both preventCancel and preventAbort works See whatwg/streams#1004 and w3ctag/promises-guide#58. -- wpt-commits: 0da3476dcd5fd3148041d090d2330cf8d412d7f9 wpt-pr: 17816
xeonchen
pushed a commit
to xeonchen/gecko
that referenced
this pull request
Jul 25, 2019
…eventCancel and preventAbort works, a=testonly Automatic update from web-platform-tests Streams: verify that aborting a pipe with both preventCancel and preventAbort works See whatwg/streams#1004 and w3ctag/promises-guide#58. -- wpt-commits: 0da3476dcd5fd3148041d090d2330cf8d412d7f9 wpt-pr: 17816
domenic
pushed a commit
to whatwg/streams
that referenced
this pull request
Jul 29, 2019
When both the preventCancel and preventAbort flags are set, the abortAlgorithm passes an empty list of actions to "wait for all". This helper should immediately resolve the returned promise when no promises are given, but instead the promise remained pending forever. This fixes the reference implementation of the "wait for all" helper to resolve correctly when given an empty list of promises. This matches the spec change in w3ctag/promises-guide#58. Originally reported in #1004 (comment).
natechapin
pushed a commit
to natechapin/wpt
that referenced
this pull request
Aug 23, 2019
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Oct 4, 2019
…eventCancel and preventAbort works, a=testonly Automatic update from web-platform-tests Streams: verify that aborting a pipe with both preventCancel and preventAbort works See whatwg/streams#1004 and w3ctag/promises-guide#58. -- wpt-commits: 0da3476dcd5fd3148041d090d2330cf8d412d7f9 wpt-pr: 17816 UltraBlame original commit: 40dd37dfbbfbc0659c81c2a386f5fe966b3c8203
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Oct 4, 2019
…eventCancel and preventAbort works, a=testonly Automatic update from web-platform-tests Streams: verify that aborting a pipe with both preventCancel and preventAbort works See whatwg/streams#1004 and w3ctag/promises-guide#58. -- wpt-commits: 0da3476dcd5fd3148041d090d2330cf8d412d7f9 wpt-pr: 17816 UltraBlame original commit: 40dd37dfbbfbc0659c81c2a386f5fe966b3c8203
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified-and-comments-removed
that referenced
this pull request
Oct 4, 2019
…eventCancel and preventAbort works, a=testonly Automatic update from web-platform-tests Streams: verify that aborting a pipe with both preventCancel and preventAbort works See whatwg/streams#1004 and w3ctag/promises-guide#58. -- wpt-commits: 0da3476dcd5fd3148041d090d2330cf8d412d7f9 wpt-pr: 17816 UltraBlame original commit: 40dd37dfbbfbc0659c81c2a386f5fe966b3c8203
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
From my comment on whatwg/streams#1004: